eclass: add ShadowedEclassPhase for multiple-eclass defined phases #762
Conversation
thesamesam
force-pushed
the
sam-inherit-pkg_setup
branch
11 times, most recently
from
2cfe29e to
91afa80
Compare
Detect the basic case of an ignored eclass phase where two eclasses are inherited, both defining the same phase, and the ebuild does not define a custom implementation of that phase at all. This means one of the exported phases from the eclasses is being ignored. Ignore some eclasses with a blacklist where they are known to vary their API by EAPI or eclass variables, at least for now, because the eclass cache we have accessible here isn't keyed by EAPI or the context of the sourcing ebuild. Bug: https://bugs.gentoo.org/516014 Bug: https://bugs.gentoo.org/795006 Closes: pkgcore#377 Signed-off-by: Sam James <sam@gentoo.org>
Signed-off-by: Sam James <sam@gentoo.org>
thesamesam
force-pushed
the
sam-inherit-pkg_setup
branch
from
91afa80 to
10489c0
Compare
|
There's a lot of noise from things like One might argue this really means we should have We can also have this be off-by-default, but nonetheless, better heuristics would be useful.. |
ferringb
left a comment
ferringb
left a comment
There was a problem hiding this comment.
random code poking. Take what makes sense, discard what doesn't. Y'all know this code better than I do.
| # We could maybe make this more finely-grained for phases we know | ||
| # are conditionally exported if this list is impacting coverage | ||
| # severely. | ||
| blacklisted_eclasses = ["pypi", "vala", "xdg"] |
There was a problem hiding this comment.
This should be a class level frozenset, so it can be varied to test in the testcases. Plus it's more obvious.
| def __init__(self, phase, providers, **kwargs): | ||
| super().__init__(**kwargs) | ||
| self.phase = phase | ||
| self.providers = tuple(providers) |
There was a problem hiding this comment.
self.providers = tuple(sorted(providers)); expect the invokers of this class will forget to sort, so just force it on them. Mark providers as typing.Iterable[str]- assuming it's a string?
And yes, I'm now brutally anal about typing. The syntax sucks, but it's better devex and I'd like to eventually enforce mypy.
| phase for phase, eclass in exported_phases.items() if len(eclass) > 1 | ||
| ) - set(defined_phases) | ||
|
|
||
| for missing in missing_custom_phases: |
There was a problem hiding this comment.
I think this code is equivalent to above.
# use the pkg's eapi, not latest. Conflicts in EAPI's the pkg isn't, don't matter for it until it is that EAPI/
possible_conflicts = set(pkg.eapi.phases_rev)
# strip out what the pkg defines
for node in bash.func_query.captures(pkg.tree.root_node).get("func", ()):
func_name = pkg.node_str(node.child_by_field_name("name"))
possible_conflicts.discard(func_name)
for phase in possible_conflicts:
# iterate the remaining phases, finding where there are multiple eclasses providing this phase.
eclass_sources = []
for eclasses, _ in inherits:
for eclass in eclasses:
# I assume .functions supports __contains__, and isn't strictly typing.Iterable[str]
if f'{eclass}_phase' in eclass.functions:
eclass_sources.append(eclass)
if len(eclass_sources) > 1:
yield ShadowedEclassPhase(phase, eclass_sources, pkg=pkg)
I'd also rename providers to eclasses in ShadowedEclassPhase to be clearer.
| # Strip out phases we already define (even if inside of those, we don't | ||
| # actually call exported phases from all eclasses inherited). Assume that | ||
| # a custom phase in the ebuild is intentionally omitting them. | ||
| missing_custom_phases = set( |
There was a problem hiding this comment.
We should also filter out eclasses where of those defining the phase, they have @PROVIDES which has the phase (because it might wrap the eclass and call that phase for you).
2 participants
Detect the basic case of an ignored eclass phase where two eclasses are
inherited, both defining the same phase, and the ebuild does not define
a custom implementation of that phase at all. This means one of the exported
phases from the eclasses is being ignored.
Ignore some eclasses with a blacklist where they are known to vary their
API by EAPI or eclass variables, at least for now, because the eclass
cache we have accessible here isn't keyed by EAPI or the context of the
sourcing ebuild.
Bug: https://bugs.gentoo.org/516014
Bug: https://bugs.gentoo.org/795006
Closes: #377
Signed-off-by: Sam James sam@gentoo.org